Add BIP352 module (take 3) #1698

josibake · 2025-07-10T08:53:21Z

This PR implements BIP352 - Silent payments. It is recommended to read through the BIP before reviewing this PR.

This is a continuation of the work in #1519 and only opened as a new PR due to the comment history on #1519 becoming quite long and difficult to sift through. It is recommended reviewers go through #1519 for background context, if interested.

include/secp256k1_silentpayments.h

josibake · 2025-07-14T15:04:20Z

Updated 6264c3d -> 9e85256 (2025_00 -> 2025_01, compare)

Added documentation for expectations around label_lookup pointer lifetimes (h/t @antonilol)
Update docs to accurately reflect that label_context is optional (h/t @antonilol)
Added a test case for passing a lookup callback with a null context (which required some small updates to the test label lookup function)

src/ctime_tests.c

real-or-random · 2025-07-21T14:15:58Z

Sorry, stopping CI here. We're about to make a release and need to the CI. :)

We'll restart the jobs here afterwards.

josibake · 2025-07-21T14:21:51Z

Update 9e85256 -> a4db279 (2025_01 -> 2025_02, compare)

Update the constant time tests to cover the _recipient_created_shared_secret and _recipient_created_output_pubkey functions (h/t @theStack )
Remove no longer needed TODO comments and clarify why a constant time test without a label lookup function is sufficient for _recipient_scan_outputs

josibake · 2025-07-22T10:29:47Z

Rebased on top of 0.7.0 release 🎉 a4db279 -> e35bede (2025_02 -> 2025_02_rebase, compare)

josibake · 2025-07-22T11:13:33Z

I did a deep dive on using (*arg)[size] in this PR and opened #1710 for discussion, since this is a broader topic than just this PR. The relevant changes for here and the downstream Bitcoin Core PRs are josibake@5a10880 and josibake/bitcoin@5835d98

src/modules/silentpayments/main_impl.h

josibake · 2025-07-24T12:23:16Z

Updated e35bede -> 1a84908 (2025_02_rebase -> 2025_03, compare)

Added a test case for the _recipient_create_output_pubkey corner case (h/t @theStack)
Removed the VERIFY_CHECK in favour of returning an error

src/modules/silentpayments/main_impl.h

examples/silentpayments.c

josibake · 2025-07-25T09:20:23Z

Update 1a84908 -> 2948a9b (2025_03 -> 2025_04, compare)

Fixed valgrind error in test
Update the example to use EXIT_SUCCESS/EXIT_FAILURE (h/t @theStack)
Clear shared secret variable consistently (and update comment) (h/t @theStack)
Add comment explaining why we declassify the pubkey sum (h/t @theStack)

Thanks for the thorough review, @theStack !

josibake · 2025-07-25T14:33:30Z

Update 2948a9b -> 64ecd6c (2025_04 -> 2025_05, compare)

Remove no longer needed TODO comment regarding _cmov
Remove todo comment regarding input_hash, now that this is properly specified in the BIP

cc @jonasnick and @real-or-random regarding the use of a VERIFY_CHECK in favour of returning an error, when returning an error results in an untestable branch. I'm happy with the approach here were we use a VERIFY_CHECK for input_hash and t_k to check for an overflow of the curve order. However, given this is something we've discussed a few times in the post, would be great to hear your thoughts on this and I'm happy to defer to whatever you both think is best.

This should address all of the outstanding TODOs (at least the ones we left comments for 😅 )

src/modules/silentpayments/bench_impl.h

josibake · 2025-07-28T18:06:21Z

Updated 64ecd6c -> 3c4af8f (2025_05 -> 2025_06, compare)

Updates the benchmarks per @theStack 's suggestion to have separate benchmarks for _full_scan and full_scan_with_labels
Leave a TODO comment for a follow-up to make the labels benchmark more representative of real world usage
Cleans up the benchmark arguments and formatting

theStack

In the CI commit, could add the silent payments module also to the native macOS arm64 job (as done for musig recently in #1699), e.g.

diff

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 8ee13ce..f612a84 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -583,13 +583,13 @@ jobs:
       fail-fast: false
       matrix:
         env_vars:
-          - { WIDEMUL: 'int64',  RECOVERY: 'yes', ECDH: 'yes', EXTRAKEYS: 'yes', SCHNORRSIG: 'yes', MUSIG: 'yes', ELLSWIFT: 'yes' }
+          - { WIDEMUL: 'int64',  RECOVERY: 'yes', ECDH: 'yes', EXTRAKEYS: 'yes', SCHNORRSIG: 'yes', MUSIG: 'yes', ELLSWIFT: 'yes', SILENTPAYMENTS: 'yes' }
           - { WIDEMUL: 'int128_struct', ECMULTGENPRECISION: 2, ECMULTWINDOW: 4 }
-          - { WIDEMUL: 'int128',                  ECDH: 'yes', EXTRAKEYS: 'yes', SCHNORRSIG: 'yes', MUSIG: 'yes', ELLSWIFT: 'yes' }
+          - { WIDEMUL: 'int128',                  ECDH: 'yes', EXTRAKEYS: 'yes', SCHNORRSIG: 'yes', MUSIG: 'yes', ELLSWIFT: 'yes', SILENTPAYMENTS: 'yes' }
           - { WIDEMUL: 'int128', RECOVERY: 'yes' }
-          - { WIDEMUL: 'int128', RECOVERY: 'yes', ECDH: 'yes', EXTRAKEYS: 'yes', SCHNORRSIG: 'yes', MUSIG: 'yes', ELLSWIFT: 'yes' }
-          - { WIDEMUL: 'int128', RECOVERY: 'yes', ECDH: 'yes', EXTRAKEYS: 'yes', SCHNORRSIG: 'yes', MUSIG: 'yes', ELLSWIFT: 'yes', CC: 'gcc' }
-          - { WIDEMUL: 'int128', RECOVERY: 'yes', ECDH: 'yes', EXTRAKEYS: 'yes', SCHNORRSIG: 'yes', MUSIG: 'yes', ELLSWIFT: 'yes', CPPFLAGS: '-DVERIFY' }
+          - { WIDEMUL: 'int128', RECOVERY: 'yes', ECDH: 'yes', EXTRAKEYS: 'yes', SCHNORRSIG: 'yes', MUSIG: 'yes', ELLSWIFT: 'yes', SILENTPAYMENTS: 'yes' }
+          - { WIDEMUL: 'int128', RECOVERY: 'yes', ECDH: 'yes', EXTRAKEYS: 'yes', SCHNORRSIG: 'yes', MUSIG: 'yes', ELLSWIFT: 'yes', SILENTPAYMENTS: 'yes', CC: 'gcc' }
+          - { WIDEMUL: 'int128', RECOVERY: 'yes', ECDH: 'yes', EXTRAKEYS: 'yes', SCHNORRSIG: 'yes', MUSIG: 'yes', ELLSWIFT: 'yes', SILENTPAYMENTS: 'yes', CPPFLAGS: '-DVERIFY' }
           - BUILD: 'distcheck'
 
     steps:

(untested)

examples/silentpayments.c

real-or-random · 2025-08-29T18:30:38Z

I pushed some (totally non-essential) fixup suggestions for the Python code to https://github.com/real-or-random/secp256k1/tree/bip352-silentpayments-module-2025-vector-fixups .

Related to this: I see why we need the ripemd160 and bech32m Python modules to parse addresses from the test vectors JSON file. This feels a bit wrong. This SP module covers some cryptographic core of SP, which does not deal with address encoding issues. So these files belong to full wallet implementations such as Bitcoin Core (where you got them from, I assume).

The only way around this that I see is that the intermediate results computed from the addresses (i.e., the extracted input pubkeys) would additionally be included in the JSON file. Do you think that would be a reasonable solution and acceptable from your point of view as a BIP author? Are there any drawbacks to this? I don't see any, but I might be missing something.

edit: Oh, or we could compute the input pubkeys from the seckeys in the JSON. Preferably in the C tests because this will require no Python EC code.

(It's not the end of the world to have this in our repo. I think this is my tendency as a maintainer to avoid code. The more code we have, the more we'll need to maintain.)

josibake · 2025-09-01T12:14:36Z

Some style things:

Thanks for the feedback! Agree with all of your points and will implement. Regarding keeping notation in line with the BIP or removing entirely, I'd prefer to avoid the BIP as much as possible. As you said, the API should be understandable without the BIP.

How the BIP and module are written today is more of a historic "oopsie." Ideally, the protocol should have been specified in a more module way, i.e., a cryptographic primitive (lets call it "non-interactive-payment", or "multiparty ECDH") and then a Bitcoin specific BIP that specifies how to use this NIP/MPECDH crypto primitive in a bitcoin specific context.

TLDR; I think we should keep this module more aligned with how things "should" have been done.

intermediate results computed from the addresses (i.e., the extracted input pubkeys) would additionally be included in the JSON file. Do you think that would be a reasonable solution and acceptable from your point of view as a BIP author? Are there any drawbacks to this? I don't see any, but I might be missing something.

Regarding the tests, I'd love to rip out all the Bitcoin specific python code. I dont see any drawbacks toward including intermediary results in the test files, and this would also be useful for index builders wishing to reuse the same set of test vectors. I've had a TODO for awhile now to update the BIP test vectors; I'll prioritise that work and update the PR here.

As always, thanks for the thorough review! I had one question that I'd like your input on, otherwise I'll work on implementing the rest of your feedback over the next couple days.

josibake · 2025-09-03T11:33:20Z

Rebased 2164d5c -> bd745e1 (2025_15 -> 2025_15-rebased, compare)

Rebased on master to include #1725

src/modules/silentpayments/main_impl.h

josibake · 2025-09-04T11:25:17Z

Updated bd745e1 -> d7281b9 (2025_15-rebased -> 2025_16, compare)

Removed the workaround for the valgrind false positive 🎉
s/public_data/prevout_summary/ (h/t @real-or-random)
Update functions that either output or take a prevout_summary object as input to be a proper lazy evaluation (h/t @real-or-random)
Avoid using BIP specific variable names, e.g., s/B_m/labeled_spend_pubkey/ etc (h/t @real-or-random)
Clean up comments to use proper punctuation and full sentences (h/t @real-or-random)
Improve error handling for malformed public keys and update test coverage (h/t @w0xlt)
Explicitly clear intermediate hash variables when creating labels (h/t @w0xlt)
Improve test coverage by explicitly testing more corner cases (h/t @real-or-random)
Fix VERIFY typo (h/t @w0xlt)

In addition to what is explicitly mentioned above, I also slightly reworded a few comments and removed comments that I felt were simply restating what the code does without adding any extra information.

Thanks for the continued review @real-or-random . I think your suggestions for variable names and comments have improved the readability quite a bit. Also much happier with the prevout_summary naming and treating the prevout_summary object as a lazy evaluation.

Also, thanks @w0xlt for the thorough review! Much happier with the error handling and glad you caught the VERIFY typo.

josibake · 2025-09-04T11:35:19Z

@real-or-random

The only way around this that I see is that the intermediate results computed from the addresses

Working on an update to the BIP test vectors to include intermediate outputs. Once I have that pull request open, I'll update the test code here to remove the bech32m and ripemd160 python code.

theStack

Re-reviewed up to the receiving commit cb53ab5, left a few suggestions only regarding code comments (i.e. typos, outdated API names/references). I agree that the new naming _prevout_summary is an improvement. 👍

include/secp256k1_silentpayments.h

src/modules/silentpayments/main_impl.h

include/secp256k1_silentpayments.h

src/modules/silentpayments/main_impl.h

Add a routine for the entire sending flow which takes a set of private keys, the smallest outpoint, and list of recipients and returns a list of x-only public keys by performing the following steps: 1. Sum up the private keys 2. Calculate the input_hash 3. For each recipient group: 3a. Calculate a shared secret 3b. Create the requested number of outputs This function assumes a single sender context in that it requires the sender to have access to all of the private keys. In the future, this API may be expanded to allow for a multiple senders or for a single sender who does not have access to all private keys at any given time, but for now these modes are considered out of scope / unsafe. Internal to the library, add: 1. A function for creating shared secrets (i.e., a*B or b*A) 2. A function for generating the "SharedSecret" tagged hash 3. A function for creating a single output public key

Add function for creating a label tweak. This requires a tagged hash function for labels. This function is used by the receiver for creating labels to be used for a) creating labeled addresses and b) to populate a labels cache when scanning. Add function for creating a labeled spend pubkey. This involves taking a label tweak, turning it into a public key and adding it to the spend public key. This function is used by the receiver to create a labeled silent payment address. Add tests for the label API.

josibake · 2025-09-05T09:10:29Z

Updated d7281b9 -> f3c0ad6 (2025_16 -> 2025_17, compare)

s/prevout_summary/prevouts_summary/ - should have done this in the last push, apologies for the churn on this
Updated tests to use the new structure introduced in BIP352: Add intermediate vector material for silent payments bitcoin/bips#1953 (h/t @macgyver13)
Various spelling/comment cleanups (h/t @theStack , I promise I'll eventually learn how to spell recipient 😅 )

The main change in this push is being able to remove the ripemd160.py and bech32m.py files from the reference code! Thanks @macgyver13 for making these changes , both here and in the BIP repo!

Add routine for scanning a transaction and returning the necessary spending data for any found outputs. This function works with labels via a lookup callback and requires access to the transaction outputs. Requiring access to the transaction outputs is not suitable for light clients, but light client support is enabled by exposing the `_create_shared_secret` and `_create_output_pubkey` functions in the API. This means the light client will need to manage their own scanning state, so wherever possible it is preferrable to use the `_recipient_scan_ouputs` function. Add an opaque data type for passing around the summed input public key (A_sum) and the input hash tweak (input_hash). This data is passed to the scanner before the ECDH step as two separate elements so that the scanner can multiply b_scan * input_hash before doing ECDH. Add functions for deserializing / serializing a public_data object to and from a public key. When serializing a public_data object, the input_hash is multplied into A_sum. This is so the object can be stored as public key for wallet rescanning later, or to vend to light clients. For the light client, a `_parse` function is added which parses the compressed public key serialization into a `public_data` object. Finally, add test coverage for the receiving API.

Demonstrate sending, scanning, and light client scanning.

Add a benchmark for a full transaction scan and for scanning a single output. Only benchmarks for scanning are added as this is the most performance critical portion of the protocol. Co-authored-by: Sebastian Falbesoner <[email protected]>

Add the BIP-352 test vectors. The vectors are generated with a Python script that converts the .json file from the BIP to C code: $ ./tools/tests_silentpayments_generate.py test_vectors.json > ./src/modules/silentpayments/vectors.h Co-authored-by: Ron <[email protected]> Co-authored-by: Sebastian Falbesoner <[email protected]> Co-authored-by: Tim Ruffing <[email protected]>

Co-authored-by: Jonas Nick <[email protected]> Co-authored-by: Sebastian Falbesoner <[email protected]>

Test midstate tags used in silent payments.

josibake · 2025-09-05T10:49:23Z

Updated f3c0ad6 -> aa85bfb (2025_17 -> 2025_18, compare)

Fix tests_silentpayments_generate.py to not skip test case (h/t @real-or-random )
Refactor tests_silentpayments_generate.py to be more idiomatic python/more readable (h/t @real-or-random )
Fix up tests_impl.h to handle extra test case (h/t @real-or-random)

Thanks for the fixup! branch for the test vectors, @real-or-random ! I added you as a coauthor.

This was referenced Jul 10, 2025

Add BIP352 silentpayments module #1519

Closed

BIP352 tracking issue bitcoin/bitcoin#28536

Open

Silent Payments: Implement BIP352 bitcoin/bitcoin#28122

Draft

antonilol reviewed Jul 10, 2025

View reviewed changes

include/secp256k1_silentpayments.h Outdated Show resolved Hide resolved

include/secp256k1_silentpayments.h Show resolved Hide resolved

josibake force-pushed the bip352-silentpayments-module-2025 branch from 6264c3d to 9e85256 Compare July 14, 2025 14:54

antonilol mentioned this pull request Jul 14, 2025

[WIP] Add bip352 silentpayments module rust-bitcoin/rust-secp256k1#721

Open

theStack reviewed Jul 21, 2025

View reviewed changes

src/ctime_tests.c Outdated Show resolved Hide resolved

josibake force-pushed the bip352-silentpayments-module-2025 branch from 9e85256 to a4db279 Compare July 21, 2025 14:09

josibake force-pushed the bip352-silentpayments-module-2025 branch from a4db279 to e35bede Compare July 22, 2025 10:27

josibake mentioned this pull request Jul 22, 2025

RFC: Using unsigned char (*arg)[size] as a replacement for unsigned char *argsize for fixed length array arguments #1710

Open

real-or-random added the feature label Jul 23, 2025

theStack reviewed Jul 23, 2025

View reviewed changes

src/modules/silentpayments/main_impl.h Outdated Show resolved Hide resolved

josibake force-pushed the bip352-silentpayments-module-2025 branch from e35bede to 1a84908 Compare July 24, 2025 12:18

theStack reviewed Jul 24, 2025

View reviewed changes

src/modules/silentpayments/main_impl.h Outdated Show resolved Hide resolved

src/modules/silentpayments/main_impl.h Outdated Show resolved Hide resolved

src/modules/silentpayments/main_impl.h Outdated Show resolved Hide resolved

examples/silentpayments.c Outdated Show resolved Hide resolved

josibake force-pushed the bip352-silentpayments-module-2025 branch from 1a84908 to 2948a9b Compare July 25, 2025 09:16

josibake force-pushed the bip352-silentpayments-module-2025 branch from 2948a9b to 64ecd6c Compare July 25, 2025 14:25

jonatack mentioned this pull request Jul 25, 2025

BIP352: be explicit for the input_hash corner case bitcoin/bips#1902

Merged

theStack reviewed Jul 28, 2025

View reviewed changes

src/modules/silentpayments/bench_impl.h Outdated Show resolved Hide resolved

josibake force-pushed the bip352-silentpayments-module-2025 branch from 64ecd6c to 3c4af8f Compare July 28, 2025 18:01

theStack mentioned this pull request Jul 30, 2025

Add ECDSA pubkey recovery usage example #1714

Open

theStack reviewed Jul 30, 2025

View reviewed changes

examples/silentpayments.c Outdated Show resolved Hide resolved

macgyver13 mentioned this pull request Aug 25, 2025

Merge bitcoin-core secp256k1 PR #1698 silent-payments/roadmap#2

Open

macgyver13 mentioned this pull request Aug 29, 2025

Merge bitcoin-core silent payments PR #28122 silent-payments/roadmap#32

Open

josibake force-pushed the bip352-silentpayments-module-2025 branch from 2164d5c to bd745e1 Compare September 3, 2025 11:31

w0xlt reviewed Sep 3, 2025

View reviewed changes

src/modules/silentpayments/main_impl.h Outdated Show resolved Hide resolved

w0xlt reviewed Sep 3, 2025

View reviewed changes

src/modules/silentpayments/main_impl.h Outdated Show resolved Hide resolved

w0xlt reviewed Sep 3, 2025

View reviewed changes

src/modules/silentpayments/main_impl.h Show resolved Hide resolved

w0xlt reviewed Sep 4, 2025

View reviewed changes

src/modules/silentpayments/main_impl.h Show resolved Hide resolved

josibake force-pushed the bip352-silentpayments-module-2025 branch from bd745e1 to d7281b9 Compare September 4, 2025 11:13

theStack reviewed Sep 4, 2025

View reviewed changes

josibake mentioned this pull request Sep 5, 2025

BIP352: Add intermediate vector material for silent payments bitcoin/bips#1953

Open

real-or-random mentioned this pull request Sep 5, 2025

Initialization of output args #1736

Open

josibake and others added 2 commits September 5, 2025 09:56

josibake force-pushed the bip352-silentpayments-module-2025 branch from d7281b9 to f3c0ad6 Compare September 5, 2025 09:05

josibake and others added 8 commits September 5, 2025 11:41

silentpayments: add examples/silentpayments.c

88eb3d4

Demonstrate sending, scanning, and light client scanning.

silentpayments: add benchmarks for scanning

038c5b9

Add a benchmark for a full transaction scan and for scanning a single output. Only benchmarks for scanning are added as this is the most performance critical portion of the protocol. Co-authored-by: Sebastian Falbesoner <[email protected]>

tests: add constant time tests

124750d

Co-authored-by: Jonas Nick <[email protected]> Co-authored-by: Sebastian Falbesoner <[email protected]>

tests: add sha256 tag test

d504e48

Test midstate tags used in silent payments.

ci: enable silentpayments module

9f42a30

docs: update README

aa85bfb

josibake force-pushed the bip352-silentpayments-module-2025 branch from f3c0ad6 to aa85bfb Compare September 5, 2025 10:44

Add BIP352 module (take 3) #1698

Are you sure you want to change the base?

Add BIP352 module (take 3) #1698

Uh oh!

Conversation

josibake commented Jul 10, 2025

Uh oh!

Uh oh!

Uh oh!

josibake commented Jul 14, 2025

Uh oh!

Uh oh!

real-or-random commented Jul 21, 2025

Uh oh!

josibake commented Jul 21, 2025

Uh oh!

josibake commented Jul 22, 2025

Uh oh!

josibake commented Jul 22, 2025

Uh oh!

Uh oh!

josibake commented Jul 24, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

josibake commented Jul 25, 2025

Uh oh!

josibake commented Jul 25, 2025

Uh oh!

Uh oh!

josibake commented Jul 28, 2025

Uh oh!

theStack left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

real-or-random commented Aug 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

josibake commented Sep 1, 2025

Uh oh!

josibake commented Sep 3, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

josibake commented Sep 4, 2025

Uh oh!

josibake commented Sep 4, 2025

Uh oh!

theStack left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

josibake commented Sep 5, 2025

Uh oh!

josibake commented Sep 5, 2025

Uh oh!

Uh oh!

real-or-random commented Aug 29, 2025 •

edited

Loading